Exclude comment sections from workspace symbols by default #866
+54
−17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jul 7, 2025
DavisVaughan
approved these changes
Jul 16, 2025
Comment on lines
+128
to
+133
| if let Some(existing_entry) = index.get(&entry.key) { | ||
| // Give priority to non-section entries. | ||
| if matches!(existing_entry.data, IndexEntryData::Section { .. }) { | ||
| index.insert(entry.key.clone(), entry); | ||
| } | ||
| // Else, ignore. |
There was a problem hiding this comment.
I think a test for this behavior would be nice
| deprecated: None, | ||
| container_name: None, | ||
| }); | ||
| if state.config.workspace_symbols.include_comment_sections { |
There was a problem hiding this comment.
Would enjoy seeing a test for this (on and off) as well if possible
| }, | ||
|
|
||
| IndexEntryData::Variable { name } => { | ||
| info.push(SymbolInformation { | ||
| name: name.clone(), |
There was a problem hiding this comment.
Could also add a nice test in goto_definition for
# my_section ----
my_section <- function() {}
to make sure that my_section is chosen
I know I'm also asking for a similar test in indexer itself, but it feels like having a duplicate test in both places is a good idea (so if we ever see the indexer test and think "why is this here?", then we'd also have the goto-definition test to show the real reason for it)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Branched from #865.
Addresses posit-dev/positron#4886.
When a comment section like
# name ---clashes with an actual symbol likename <- function() {}, whichever symbols comes first (usually the section) is included in the workspace index, and the other is ignored. Ideally we'd track both but for now we only track one identifier per file.This causes sections to win over function definitions in cases like this:
If you then jump to definition on a reference to that function, Positron jumps to the section instead of the function.
To fix this, we now allow functions and variables to overwrite sections in the indexer.
In addition, we no longer emit sections as workspace symbols by default. This can be turned back on with a new setting
positron.r.workspaceSymbols.includeCommentSections. This is consistent with the fix in Exclude section headers from workspace symbols in R packages quarto-dev/quarto#755 where we no longer export markdown headers as workspace symbols by default, to avoid flooding workspace symbol quickpicks (#prefix in command palette) with section headers.QA Notes
You should be able to command+quick on the
my_sectionreference and be taken to the function definition rather than the section. This should be the case no matter the value ofpositron.r.workspaceSymbols.includeCommentSections.When
positron.r.workspaceSymbols.includeCommentSectionsis set totrue, you should see comment sections in the workspace symbol quickpick. Otherwise they shouldn't be included.